Scope the likes collection group to the requesting user - #184
Merged
Conversation
`collectionGroup('likes').where('userId','==',victim)` returned anyone's
complete like history — every post they have ever liked, in one query, to an
unauthenticated caller. The composite index exists, so it was servable today.
Per-post likes stay world-readable and that is deliberate: a post's like count
and its likers are public, and that is the only shape any UI needs. The
collection group answers a different question — "every like matching a filter,
across all posts" — and no UI has ever exposed that aggregate. "These 40
accounts liked this photo" and "here is everything this person has ever liked"
are different facts about someone.
The only client that uses the collection group is useBatchLikeStatus, which
already filters by the signed-in user's own uid, so it is unaffected. There is
a test pinning exactly its query shape (userId == me AND postId in [...]) so
this cannot be tightened further by accident and silently strip like state off
every feed card.
Three new tests fail on the old rule and pass on this one: the unauthenticated
cross-post query, a signed-in user reading someone else's history, and an
unfiltered scan. Two more pass on both, as guardrails: an unauthenticated read
of a single post's likes, and the batched own-likes query.
Checkins were reported alongside this and are NOT changed here — the same
tightening would break the public per-pet check-in history on PetProfile, and
constraining only the userId path would not close the exposure anyway. Raised
separately rather than half-fixed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
collectionGroup('likes').where('userId','==',victim)returned anyone's complete like history — every post they have ever liked, in one query, to an unauthenticated caller. The composite index exists, so it was servable today.Why per-post likes stay public
Two different rules, two different questions:
posts/{postId}/likesread: if true{path=**}/likes(CG)"These 40 accounts liked this photo" and "here is everything this person has ever liked" are different facts about someone. No UI has ever exposed the second one.
Blast radius: none
The only client that touches the collection group is
useBatchLikeStatus, which already filters by the signed-in user's own uid. There's a test pinning its exact query shape —userId == me AND postId in [...]— so a future tightening can't silently strip like state off every feed card.Tests
tests/rules/ban-and-engagement.test.ts, 25 → 30.Three fail on the old rule, pass on this one:
Two pass on both, as guardrails: an unauthenticated read of a single post's likes, and the batched own-likes query the feed depends on.
Local run: rules 62/62.
Checkins are NOT in this PR — and the reason matters
The audit reported checkins alongside likes, and the instruction was to constrain both. I did not, because constraining
checkinsto the requesting user breaks a shipped feature, and doing only theuserIdhalf would not close the exposure anyway. Details in the PR discussion — this needs a decision, not a guess.Deploy
🤖 Generated with Claude Code